Skip to content

fix(rpc): resolve EthTxIndex -1 sentinel before uint cast in receipt builder#1047

Open
Aboudjem wants to merge 3 commits intocosmos:mainfrom
Aboudjem:fix/ethtxindex-sentinel
Open

fix(rpc): resolve EthTxIndex -1 sentinel before uint cast in receipt builder#1047
Aboudjem wants to merge 3 commits intocosmos:mainfrom
Aboudjem:fix/ethtxindex-sentinel

Conversation

@Aboudjem
Copy link

@Aboudjem Aboudjem commented Mar 4, 2026

Description

Closes: #1032

Summary

When TxResult.EthTxIndex is -1 (sentinel for "not yet assigned by indexer"), ReceiptsFromCometBlock casts it directly to uint, wrapping to 18446744073709551615 (MaxUint64). This breaks all modern Ethereum JS clients (viem, wagmi, ethers.js) with IntegerOutOfRangeError.

Fix

Resolve the -1 sentinel using the loop index before the uint cast, matching the existing pattern in GetTransactionByHash (tx_info.go):

```go
if txResult.EthTxIndex == -1 {
txResult.EthTxIndex = int32(i)
}
```

Testing

  • Verified receipts return correct transactionIndex values
  • All existing tests pass

Author Checklist

  • tackled an existing issue or discussed with a team member
  • left instructions on how to review the changes
  • targeted the main branch

@aljo242
Copy link
Contributor

aljo242 commented Mar 4, 2026

Please test this functionality. We require testing on all of our contributions.

@Aboudjem
Copy link
Author

Aboudjem commented Mar 4, 2026

Tests added in rpc/backend/tx_info_test.go.

@codecov
Copy link

codecov bot commented Mar 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.66%. Comparing base (64c7499) to head (93f8d1c).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1047      +/-   ##
==========================================
+ Coverage   64.64%   64.66%   +0.01%     
==========================================
  Files         319      319              
  Lines       22170    22172       +2     
==========================================
+ Hits        14332    14337       +5     
+ Misses       6718     6715       -3     
  Partials     1120     1120              
Files with missing lines Coverage Δ
rpc/backend/comet_to_eth.go 81.76% <100.00%> (+0.20%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vladjdk
Copy link
Member

vladjdk commented Mar 5, 2026

lints failing @Aboudjem

@Aboudjem
Copy link
Author

Aboudjem commented Mar 6, 2026

lints failing @Aboudjem

done @vladjdk

@Aboudjem Aboudjem force-pushed the fix/ethtxindex-sentinel branch from d4cc927 to c195377 Compare March 11, 2026 23:20
@Aboudjem
Copy link
Author

Rebased on latest main. CI is green — ready for review when you have a chance. cc @vladjdk @aljo242

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: EthTxIndex -1 sentinel wraps to MaxUint64 in ReceiptsFromCometBlock, breaking JS clients

3 participants